-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parse browser compatibility metadata #144
Conversation
How does the annotation help end users of this package? I love the idea of using this data to prune the output. |
It'll probably depend on what we choose for our criteria to emit or exclude generation of APIs. If they're all the same (supports standards track and all three browsers) than the metadata won't provide any real value. But the APIs currently support the full gamut of maturity and browser support:
|
Not standards track:
no supported browsers:
only one supported browser:
missing one browser (2 of 3 supported):
|
Upleveling this a bit, I think the important things are:
That 2nd point could be via annotations, a table in the generated docs, ... |
There is a push amongst from W3C and all the major browser teams to standardise some of that comparability data via “Baseline” which as an end user I absolutely would find super helpful to know about in the code. |
Thanks for the pointer! I took a look at baseline; the information in there looks less comprehensive than what we have in browser-compat-data today. But perhaps baseline will evolve to be the best system-of-record for the info? In any case, we're likely going to look at what the best system to record for this info is (baseline? browser-compat-data? the w3c specs themselves?) and use that to evolve how many of these APIs that we generate, and whether we generate any w/ some experimental verbiage. Moving this PR to a draft as we're less likely to land this specific implementation. |
@srujzs - this PR has been updated to parse the browser compat metadata but not to use it during generation (it no longer generates annotations w/ the data). |
Cool, thanks! Is this ready for review or are you iterating on it still? |
It's ready for review now - |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Mostly just some nits.
tool/generator/bcd.dart
Outdated
properties = Map.fromIterable( | ||
names, | ||
value: (key) => BCDPropertyStatus( | ||
key as String, json[key] as Map<String, dynamic>, this), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MDN exposes compatibility info for dictionary interfaces with an _option
suffix e.g. https://github.com/mdn/browser-compat-data/blob/cfa15e085ceb88cdee391ae09bd52ee3674d5ecc/api/MediaDevices.json#L264C10-L264C29. We could handle this here by having a separate "dictionaryOptions" field in BCDInterfaceStatus
or we can just do the lookup in the generation script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, interesting. This would be metadata about parameters for an interface's method?
It looks like these options exist off properties? So we'd want BCDInterfaceStatus
to contain BCDPropertyStatus
items, and for those items to have optional options
(anything w/ an _option
suffix).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to help me think about the organization in the bcd.json file, there are ~16 foo_option
instances in there (all fields off of interface properties?).
https://gist.github.com/devoncarew/81c987aa55637b6014c5b400a7edf022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funnily enough, that info is actually a separate thing altogether and I think they're suffixed with _parameter
: https://github.com/mdn/browser-compat-data/blob/c2761cb84874ff5b9b0a230e7a557a5a43b9b939/api/EventTarget.json#L160C10-L160C27. :D Also see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#browser_compatibility.
This is for what options can go in an options object that is passed to an API. https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getDisplayMedia#browser_compatibility
There's also "_parameter_optional" apparently for optional parameters: https://github.com/mdn/browser-compat-data/blob/c2761cb84874ff5b9b0a230e7a557a5a43b9b939/api/HTMLTableRowElement.json#L378.
I think we may want to validate all the strings and check for _
to make sure we're handling the various types of compatibility info. There's also some guidance here: https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines/index.md#parameters-and-parameter-object-features, but it doesn't seem to catch every variant unfortunately. Realistically, there are probably not enough APIs that use all the variants for this to matter from our perspective, but something to watch out for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, taking another pass through this, it looks like neither _option
nor _parameter
show up in the first two levels of the data (the interfaces and their properties).
Separately, I do see some types we're generating interop code for that aren't represented in the BCD data. I expect we'll need to adjust how we parse and use the data as we start to filter how much of the IDL we generate code for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, taking another pass through this, it looks like neither _option nor _parameter show up in the first two levels of the data (the interfaces and their properties).
Right, we can do this in another CL to grab more information about a specific member or constructor.
Separately, I do see some types we're generating interop code for that aren't represented in the BCD data. I expect we'll need to adjust how we parse and use the data as we start to filter how much of the IDL we generate code for.
I suspect the majority of these cases to be because the types are new and are still being drafted, and therefore MDN doesn't have information on them. It'll be useful to validate that, but I suspect if the type is not present, we can safely omit the type.
See also https://github.com/mdn/browser-compat-data.
This PR does not use the info currently; follow-ups to this PR should likely use this info to reduce the amount of API that we generate (i.e., don't generate an API that does not support standards track; don't generate an API w/ no supported browsers; consider not generating an API w/ only one supported browser).
Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.